Multi-touch gesture support #682
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on December 20
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
Test Results28 tests 28 ✅ 1m 31s ⏱️ Results for commit 6b0dd59. ♻️ This comment has been updated with latest results. |
PetrESP
force-pushed
the
petr/lvgl_multi_touch
branch
from
05aa5f5 to
cd3dd64
Compare
PetrESP
force-pushed
the
petr/lvgl_multi_touch
branch
2 times, most recently
from
ad4bf26 to
363d040
Compare
PetrESP
force-pushed
the
petr/lvgl_multi_touch
branch
from
363d040 to
221ef74
Compare
tore-espressif
left a comment
tore-espressif
left a comment
There was a problem hiding this comment.
@PetrESP Great work! We can go through it together tomorrow
| assert(tp != NULL); | ||
| assert(track_id != NULL); | ||
| assert(point_num > 0); |
There was a problem hiding this comment.
Since this is a public API, I'd recommend returning an error if any of the input arguments is invalid.
Otherwise we get abort() call in debug configuration. However, in release configuration assert() is resolved to No-Operation, which could lead to hard faults later in the code
| * @param tp: Touch handler | ||
| * @param track_id: Array of track ids | ||
| * @param point_num: Count of touched points to return |
There was a problem hiding this comment.
Nitpick, but useful. It wasn't clear to me from the first look. Also the parameter description could be made clearer...
* @param[in] tp: Touch handler
* @param[out] track_id: Array of track ids
* @param[in] point_num: Count of touched points to return
There was a problem hiding this comment.
Is it supported by Doxygen? I am not sure - the task with Doxygen warnings is still open :-(
There was a problem hiding this comment.
I've check and I don't think that it would even matter for doxygen. We do not generate any documentation from esp_lcd_touch. We only generate API files for BSPs where we add the direction of each argument. So maybe I could do it anyway to improve clarity of these comments?
| @@ -1,4 +1,4 @@ | |||
| version: "1.1.3" | |||
| version: "1.1.4" | |||
There was a problem hiding this comment.
We must bump esp_lcd_touch dependecy to at least 1.1.3 on line 7, because we use the new multi touch API that is introduced in esp_lcd_touch 1.1.3
| ### Detecting gestures | ||
|
|
||
| LVGL includes support for software detection of multi-touch gestures. | ||
| This detection can be enabled by setting the `LVGL_PORT_ENABLE_GESTURES` config and having `ESP_LCD_TOUCH_MAX_POINTS` > 1. |
There was a problem hiding this comment.
Do we need this LVGL_PORT_ENABLE_GESTURES config? What about enable it when it is enabled in LVGL - LV_USE_GESTURE_RECOGNITION + touch controller supports it.
There was a problem hiding this comment.
I've enabled it for LV_USE_GESTURE_RECOGNITION in all cases. This option will need to be manually configured in sdkconfig so I think it is sufficient.
| /* Initialize LVGL touch data for each activated touch point */ | ||
| /* static */ lv_indev_touch_data_t touches[CONFIG_ESP_LCD_TOUCH_MAX_POINTS] = {0}; | ||
| if (err != ESP_ERR_NOT_SUPPORTED) { | ||
| for (int i = 0; i < touchpad_cnt; i++) { |
There was a problem hiding this comment.
I would like to see some assert (or check) that touchpad_cnt <= CONFIG_ESP_LCD_TOUCH_MAX_POINTS
There was a problem hiding this comment.
Please check the latest change
| * @param tp: Touch handler | ||
| * @param track_id: Array of track ids | ||
| * @param point_num: Count of touched points to return |
There was a problem hiding this comment.
Is it supported by Doxygen? I am not sure - the task with Doxygen warnings is still open :-(
PetrESP
force-pushed
the
petr/lvgl_multi_touch
branch
from
221ef74 to
6dd41de
Compare
|
PetrESP
force-pushed
the
petr/lvgl_multi_touch
branch
from
6dd41de to
797fda1
Compare
PetrESP
force-pushed
the
petr/lvgl_multi_touch
branch
from
797fda1 to
e10be6a
Compare
PetrESP
force-pushed
the
petr/lvgl_multi_touch
branch
from
e10be6a to
538c156
Compare
PetrESP
force-pushed
the
petr/lvgl_multi_touch
branch
from
538c156 to
6b0dd59
Compare
|
|
|
|
|
|
|
|
ESP-BSP Pull Request checklist
Change description
Note
Introduces multi-touch gesture support in LVGL port backed by a new structured touch data API, updates drivers to provide track IDs, and refreshes examples/docs and component versions.
LVGL Port (
components/esp_lvgl_port)esp_lcd_touch_get_data; adds timestamps; docs describe enabling/config and thresholds.2.7.0with changelog.Touch Core (
components/lcd_touch/esp_lcd_touch)esp_lcd_touch_get_data+esp_lcd_touch_point_data_t(x, y, strength, track_id);get_coordinatesdeprecated.get_track_idin driver vtable.ESP_RETURN_ON_FALSE.1.2.0.Touch Drivers
track_id; integrates new checks; version1.2.0.points_data[]access; minor validation updates; versions bumped.Examples (
examples/display)CONFIG_LV_USE_GESTURE_RECOGNITIONand float in defconfig.BSP (
bsp/m5dial)esp_lcd_touch_ft5x06path; README benchmark/date and LVGL version updated to 9.4 with new metrics.Written by Cursor Bugbot for commit 6b0dd59. This will update automatically on new commits. Configure here.